Heap<Value> doesn't root anything on its own. For this to be safe, it needs to be on a traced Heap<T>. So I'm pretty sure this isn't safe.

Heap<Value> doesn't root anything on its own. For this to be safe, it needs to be on a traced Heap<T>. So I'm pretty sure this isn't safe.

So if I understand correctly, as_handle_value Is only safe If it's stored inside a structure that is traced by SpiderMonkey (e.g. inside a RootedGuard, JS::Rooted or custom traceable struct) I can rename it to AsHandleValueIfRooted with more detailed doc.

context:
We need this trait to safely convert a rooted Heap into a HandleValue<'a> without repeating unsafe { HandleValue::from_raw(self.handle()) } at every call site

I'm not a fan of this solution because it doesn't actually force the caller to use it in a safe way; it just hides an unsafe conversion.

I think if we annotate Heap with the crown::unrooted_must_root so that we can have unrooted values be compiler errors I would be willing to reevaluate this.

I would also prefer creating the rust::Handle values from the Heap values directly so the lifetime comes from the self value. Handle::from_raw is really unsafe because the lifetime has no relationship with the original source.

I think if we annotate Heap with the crown::unrooted_must_root so that we can have unrooted values be compiler errors I would be willing to reevaluate this.

Wouldn't that be to strict in some cases?

I think if we annotate Heap with the crown::unrooted_must_root so that we can have unrooted values be compiler errors I would be willing to reevaluate this.

Wouldn't that be to strict in some cases?

I'm having trouble coming up with those cases.

I would also prefer creating the rust::Handle values from the Heap values directly so the lifetime comes from the self value. Handle::from_raw is really unsafe because the lifetime has no relationship with the original source.

In this case we should put the trait in servo, as we need crown to ensure it's safety.

I would also prefer creating the rust::Handle values from the Heap values directly so the lifetime comes from the self value. Handle::from_raw is really unsafe because the lifetime has no relationship with the original source.

I will do that in Servo.

I think if we annotate Heap with the crown::unrooted_must_root so that we can have unrooted values be compiler errors I would be willing to reevaluate this.

sounds good.